Repository navigation
Conversation
New sketches use the cross-language Count-Min format, version 2, defined in the TCK (docs/count-min-format.md): - row seeds: rowSeed[i] = XXH64(LE64(i), seed) - items hashed with XxHash64: longs as 8 little-endian bytes, strings as UTF-8, byte arrays as given - bucket = (hash >>> 1) % numBuckets - serial version byte 2; the byte layout is unchanged A sketch read from a version 1 image keeps the version 1 hashing (Random seeds, MurmurHash3, floorMod) and is written back as version 1. Merging version 1 with version 2 throws, and other serial versions are rejected. A null byte array is now ignored, like an empty one. Tests check every test vector in the spec (XXH64, row seeds, buckets and both images), and the version 1 path against an independent computation of the legacy hashing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- CountMinSketch had no public way to create a sketch; only deserialize was public. Add public constructors (numHashes, numBuckets) with the default seed 9001, and (numHashes, numBuckets, seed). - Add CountMinSketchCrossLanguageTest, which generates and checks the version 2 CL files defined in section 8 of the TCK spec: count_min_v2_<case>_<language>.sk, for cases n0, n1, n10, n100, n1000, n10000 and seed42_n1000. Files from another language must also be byte-identical to Java's. The C++ version 1 files are checked to load as version 1 sketches. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Each case from section 8.1 of the spec now states its name, seed and N directly, so the test no longer parses N out of the case name. This also clears CodeQL alert 995 (possible uncaught NumberFormatException). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
proost
reviewed
Nov 11, 2026
| * The product of numHashes and numBuckets must be less than 2<sup>30</sup>. | ||
| * @throws SketchesArgumentException if a parameter is out of range | ||
| */ | ||
| public CountMinSketch(final byte numHashes, final int numBuckets) { |
Member
There was a problem hiding this comment.
int type can be more readable to user than byte type for numHashes. what is your intention?
Member
Author
There was a problem hiding this comment.
Good point, thanks. Both public constructors now take int numHashes, checked to be from 1 to 127. It is cast to a byte only where it is stored, since the image keeps it in one byte. So new CountMinSketch(3, 1000) works without a cast, and a byte from suggestNumHashes() still passes directly. getNumHashes_() and the serialization format are unchanged. A new test covers the range (0, -1, 128 and 259 are rejected; 127 round-trips).
| * @param seed The base hash seed | ||
| * @throws SketchesArgumentException if a parameter is out of range | ||
| */ | ||
| public CountMinSketch(final byte numHashes, final int numBuckets, final long seed) { |
Member
Author
There was a problem hiding this comment.
Changed here too. See the reply on the first constructor above.
numHashes is checked to be from 1 to 127 and cast to a byte only where it is stored, so callers no longer need to write (byte) 3. A byte from suggestNumHashes() still passes directly. getNumHashes_(), the image format and deserialization are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This implements the cross-language Count-Min serialization format, version 2, specified in apache/datasketches-tck#21 (
docs/count-min-format.md) and agreed in apache/datasketches-tck#18.Please don't merge this until apache/datasketches-tck#21 has merged, because the class javadoc links to the spec there. It's open now so the other implementations can compare against it.
Version 2
rowSeed[i] = XXH64(LE64(i), seed).(hash >>> 1) % numBuckets.Random, MurmurHash3,floorMod) and is written back as version 1.nullbyte array is ignored like an empty one, as the spec requires. Before, it threw aNullPointerException.Public constructors
Until now,
CountMinSketchhad no public way to create a sketch; onlydeserializewas public. This adds:CountMinSketch(byte numHashes, int numBuckets), with the default seed 9001;CountMinSketch(byte numHashes, int numBuckets, long seed).Both take the values returned by
suggestNumHashesandsuggestNumBucketsdirectly.Tests
CountMinSketchTestchecks every test vector in section 7 of the spec:It also checks:
CountMinSketchCrossLanguageTestgenerates and checks the version 2 CL files from section 8 of the spec:count_min_v2_<case>_<language>.sk;The full suite passes (2,393 tests).
🤖 Generated with Claude Code